Skip to content

Conversation

flovent
Copy link
Contributor

@flovent flovent commented Oct 7, 2025

For CompoundAssignOperator in condition, there will be two layers of ImplicitCastExpr, for code:

int val = -1;
while(val >>= 7) {
}

While statement's AST:

WhileStmt <line:4:5, line:5:5>
      |-ImplicitCastExpr <line:4:11, col:18> 'bool' <IntegralToBoolean>
      | `-ImplicitCastExpr <col:11, col:18> 'int' <LValueToRValue>
      |   `-CompoundAssignOperator <col:11, col:18> 'int' lvalue '>>=' ComputeLHSTy='int' ComputeResultTy='int'
      |     |-DeclRefExpr <col:11> 'int' lvalue Var 0x20290cb8 'val' 'int'
      |     `-IntegerLiteral <col:18> 'int' 7
      `-CompoundStmt <col:21, line:5:5>

This is not taken into account by the check when determining whether brackets need to be added.

Closes #161318.

@llvmbot
Copy link
Member

llvmbot commented Oct 7, 2025

@llvm/pr-subscribers-clang-tidy

@llvm/pr-subscribers-clang-tools-extra

Author: None (flovent)

Changes

For CompoundAssignOperator in condition, there will be two layers of ImplicitCastExpr, for code:

int val = -1;
while(val &gt;&gt;= 7) {
}

While statement's AST:

WhileStmt &lt;line:4:5, line:5:5&gt;
      |-ImplicitCastExpr &lt;line:4:11, col:18&gt; 'bool' &lt;IntegralToBoolean&gt;
      | `-ImplicitCastExpr &lt;col:11, col:18&gt; 'int' &lt;LValueToRValue&gt;
      |   `-CompoundAssignOperator &lt;col:11, col:18&gt; 'int' lvalue '&gt;&gt;=' ComputeLHSTy='int' ComputeResultTy='int'
      |     |-DeclRefExpr &lt;col:11&gt; 'int' lvalue Var 0x20290cb8 'val' 'int'
      |     `-IntegerLiteral &lt;col:18&gt; 'int' 7
      `-CompoundStmt &lt;col:21, line:5:5&gt;

This is not taken into account by the check when determining whether brackets need to be added.

Closes #161318.


Full diff: https://github.com/llvm/llvm-project/pull/162215.diff

3 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp (+2-1)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5)
  • (modified) clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp (+10)
diff --git a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
index 3fb856097a7e9..bfdf9cbd92d2b 100644
--- a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
@@ -89,7 +89,8 @@ static void fixGenericExprCastToBool(DiagnosticBuilder &Diag,
 
   const Expr *SubExpr = Cast->getSubExpr();
 
-  bool NeedInnerParens = utils::fixit::areParensNeededForStatement(*SubExpr);
+  bool NeedInnerParens =
+      utils::fixit::areParensNeededForStatement(*SubExpr->IgnoreImpCasts());
   bool NeedOuterParens =
       Parent != nullptr && utils::fixit::areParensNeededForStatement(*Parent);
 
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 7e836a7114d50..ab73456782faa 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -381,6 +381,11 @@ Changes in existing checks
   declarations and macros in system headers. The documentation is also improved
   to differentiate the general options from the specific ones.
 
+- Improved :doc:`readability-implicit-bool-conversion
+  <clang-tidy/checks/readability/implicit-bool-conversion>` check by correctly
+  adding parentheses when the inner expression are implicitly converted
+  multiple times.
+
 - Improved :doc:`readability-qualified-auto
   <clang-tidy/checks/readability/qualified-auto>` check by adding the option
   `IgnoreAliasing`, that allows not looking at underlying types of type aliases.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp
index f3e8bf044b31b..a0e1fd309768c 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp
@@ -547,3 +547,13 @@ namespace PR71848 {
 // CHECK-FIXES: return static_cast<int>( foo );
   }
 }
+
+namespace PR161318 {  
+  int AddParenOutsideOfCompoundAssignOp() {
+    int val = -1;
+    while(val >>= 7) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: implicit conversion 'int' -> 'bool' [readability-implicit-bool-conversion]
+    // CHECK-FIXES: while((val >>= 7) != 0) {
+    }
+  }
+}

@flovent flovent changed the title [clang-tidy] Fix false positives about CompoundAssignOperator in readability-implicit-bool-conversion [clang-tidy] Correctly add parentheses in readability-implicit-bool-conversion Oct 7, 2025
Copy link
Contributor

@vbvictor vbvictor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[clang-tidy] Incorrect autofix ignoring the correct precedence
3 participants